Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

random fixes that help vite/sveltekit #3140

Merged
merged 7 commits into from
Jun 1, 2023
Merged

Conversation

paperdave
Copy link
Member

@paperdave paperdave commented May 31, 2023

Status — hello worlds work, but real applications including the sveltekit demo do not fully work

This PR makes progress towards #250 and #600, fixing small random bugs in places to help vite and sveltekit run within bun's runtime:

  • fs: existsSync(null) and other invalid paths must not throw
  • resolver: import "file:///..."
  • Path.isAbsoluteString supports utf16 strings
  • unrelated, make Bun.readableStreamToArrayBuffer sync if possible.

this also (temporarily) contains changes from #2913 which I think help out vite

@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

prettier errors have been resolved. Thank you.

#dbb5ab320b7de2ea5316c30ae056beb0b243489c

@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

@paperdave 2 files with test failures on linux-x64-baseline:

  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/node/child_process/child_process-node.test.js

View test output

#dbb5ab320b7de2ea5316c30ae056beb0b243489c

@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

@paperdave 2 files with test failures on linux-x64:

  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/node/child_process/child_process-node.test.js

View test output

#7c79ddfa0106e8beee33b6458aa6b20144886d3c

@github-actions
Copy link
Contributor

github-actions bot commented May 31, 2023

@paperdave 4 files with test failures on bun-darwin-x64-baseline:

  • test/js/bun/spawn/spawn-streaming-stdin.test.ts
  • test/js/bun/sqlite/sqlite.test.js
  • test/js/bun/util/sleepSync.test.ts
  • test/js/web/timers/setTimeout.test.js

View test output

#7c79ddfa0106e8beee33b6458aa6b20144886d3c

@paperdave paperdave marked this pull request as ready for review June 1, 2023 22:22
@@ -1129,6 +1129,41 @@ pub const Resolver = struct {
return .{ .not_found = {} };
}

if (strings.hasPrefixComptime(import_path, "file:///")) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't handle space characters or anything percent-encoded

but it is better than status quo

return Exists{
.path = path,
.path = PathLike.fromJS(ctx, arguments, exception),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this fix, it should be handled earlier in the function instead of these extra null checks.

But it does fix the issue. Please add a TODO note to address this.

@Jarred-Sumner Jarred-Sumner merged commit 42d8b71 into main Jun 1, 2023
@Jarred-Sumner Jarred-Sumner deleted the dave/random-fixes branch June 1, 2023 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants